Skip to content

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Oct 30, 2023

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

In the cortex-debug section of the debug info, all the values are converted by default to a string in the resulting JSON.
Now, If another type is needed, the value can be prefixed with the tags [boolean], [number], or [object] to force a specific type in the JSON, for example:

debug.cortex-debug.custom.aBoolean=[boolean]true
debug.cortex-debug.custom.anNumber=[number]10
debug.cortex-debug.custom.anotherNumber=[number]10.20
debug.cortex-debug.custom.anObject=[object]{"key":"value", "boolean":true}

will result in the following JSON:

{
  "aBoolean": true,
  "anNumber": 10,
  "anotherNumber": 10.2,
  "anObject": {
    "boolean": true,
    "key": "value"
  }
}

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change, and is titled accordingly?

Other information

@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (ef72bde) 64.48% compared to head (80fec99) 64.51%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2393      +/-   ##
==========================================
+ Coverage   64.48%   64.51%   +0.03%     
==========================================
  Files         207      207              
  Lines       19592    19630      +38     
==========================================
+ Hits        12633    12665      +32     
- Misses       5867     5871       +4     
- Partials     1092     1094       +2     
Flag Coverage Δ
unit 64.51% <95.08%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
commands/debug/debug_info.go 71.70% <95.08%> (+4.64%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@umbynos umbynos added this to the Arduino CLI v0.35.0 milestone Oct 30, 2023
@umbynos umbynos assigned umbynos and cmaglie and unassigned umbynos Oct 30, 2023
@cmaglie cmaglie marked this pull request as ready for review October 30, 2023 17:22
@cmaglie cmaglie added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Oct 30, 2023
@kittaakos
Copy link
Contributor

Will this work with arrays? If yes, please update the example in the PR description. Thanks!

I am going to use this version downstream: arduino/vscode-arduino-tools#41.

@cmaglie
Copy link
Member Author

cmaglie commented Oct 31, 2023

I've upgraded the conversion process, now it can:

  • build JSON object via properties hierarchy
  • allow any type in array elements (even other arrays recursively)

This change should make it possible to build any JSON from properties.

@cmaglie cmaglie merged commit 64f1853 into arduino:master Oct 31, 2023
@cmaglie cmaglie deleted the debug_types branch October 31, 2023 13:34
cmaglie added a commit that referenced this pull request Oct 31, 2023
* debug: Allow type-specification of JSON output for cortex-debug

* Improved JSON properties generation
@cmaglie
Copy link
Member Author

cmaglie commented Oct 31, 2023

@kittaakos I've cherrypicked this PR in the 0.35.x branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants